Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs(5.x): update svg.md #953

Merged
merged 4 commits into from
Feb 9, 2025
Merged

docs(5.x): update svg.md #953

merged 4 commits into from
Feb 9, 2025
+23 −27

Conversation

szymonrybczak
Copy link
Contributor

@szymonrybczak szymonrybczak commented Feb 5, 2025 •
edited
Loading

Summary

Closes #928

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 312a00f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel Vercel
Copy link

vercel bot commented Feb 5, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
repack-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 6, 2025 6:39pm

@szymonrybczak
Copy link
Contributor Author

btw regarding svgs, i'd remove this section from the default config:

repack/templates_v5/rspack.config.cjs

Lines 128 to 137 in 0bc5e55

/**
* This loader handles all static assets (images, video, audio and others), so that you can
* use (reference) them inside your application.
*
* If you want to handle specific asset type manually, filter out the extension
* from `ASSET_EXTENSIONS`, for example:
* ```
* Repack.ASSET_EXTENSIONS.filter((ext) => ext !== 'svg')
* ```
*/

it makes it unnecessary long.

jbroma
jbroma reviewed Feb 6, 2025
View reviewed changes
Copy link
Member

@jbroma jbroma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 more nit: Webpack -> webpack according to the webpack branding guidelines

website/src/5.x/docs/guides/svg.md Outdated Show resolved Hide resolved
website/src/5.x/docs/guides/svg.md Outdated Show resolved Hide resolved
website/src/5.x/docs/guides/svg.md Outdated Show resolved Hide resolved
website/src/5.x/docs/guides/svg.md Outdated Show resolved Hide resolved
szymonrybczak and others added 2 commits February 6, 2025 14:57
@szymonrybczak @jbroma
Update svg.md
19f0f28 
Co-authored-by: Jakub Romańczyk <[email protected]>
@szymonrybczak @jbroma
Update svg.md
d3955af 
Co-authored-by: Jakub Romańczyk <[email protected]>
@szymonrybczak szymonrybczak requested a review from jbroma February 6, 2025 18:39
jbroma
jbroma approved these changes Feb 9, 2025
View reviewed changes
Copy link
Member

@jbroma jbroma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@jbroma
Copy link
Member

jbroma commented Feb 9, 2025

btw regarding svgs, i'd remove this section from the default config:

repack/templates_v5/rspack.config.cjs

Lines 128 to 137 in 0bc5e55

/**
* This loader handles all static assets (images, video, audio and others), so that you can
* use (reference) them inside your application.
*
* If you want to handle specific asset type manually, filter out the extension
* from `ASSET_EXTENSIONS`, for example:
* ```
* Repack.ASSET_EXTENSIONS.filter((ext) => ext !== 'svg')
* ```
*/

it makes it unnecessary long.

yeah, we will tackle this in another PR 👍

@jbroma jbroma merged commit 5621774 into main Feb 9, 2025
2 checks passed
@jbroma jbroma deleted the docs/svg branch February 9, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbroma jbroma jbroma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Docs] Guides: SVG
2 participants
@szymonrybczak @jbroma